Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opt-in querying of non-leaders #709

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

cole-miller
Copy link
Contributor

@cole-miller cole-miller commented Oct 2, 2024

This PR is a first stab at allowing non-leader nodes to serve PREPARE, QUERY, and QUERY_SQL requests. When served by a non-leader, such requests will in general return stale data (QUERY, QUERY_SQL), or fail unhelpfully (PREPARE and QUERY_SQL, see #395). But when these limitations are tolerable, the option to query a non-leader can be helpful to spread load or (when targeting the local node) avoid network round-trips.

Clients must opt into relaxing the current leader checks on a per-connection basis. This is done by repurposing the old, unused flags field on the OPEN request (which non-leaders already accept). We relax the leader checks when the least-significant bit is set in flags; this is backwards-compatible since extant versions of go-dqlite always set this field to 0. The specific relaxations are:

Signed-off-by: Cole Miller cole.miller@canonical.com

Signed-off-by: Cole Miller <cole.miller@canonical.com>
Signed-off-by: Cole Miller <cole.miller@canonical.com>
Signed-off-by: Cole Miller <cole.miller@canonical.com>
Copy link

codecov bot commented Oct 2, 2024

Codecov Report

Attention: Patch coverage is 92.70073% with 10 lines in your changes missing coverage. Please review.

Project coverage is 81.01%. Comparing base (06992a6) to head (384712f).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/gateway.c 84.12% 2 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #709      +/-   ##
==========================================
+ Coverage   80.95%   81.01%   +0.05%     
==========================================
  Files         196      196              
  Lines       29186    29287     +101     
  Branches     4089     4090       +1     
==========================================
+ Hits        23627    23726      +99     
- Misses       3895     3899       +4     
+ Partials     1664     1662       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cole-miller
Copy link
Contributor Author

cole-miller commented Oct 3, 2024 via email

Signed-off-by: Cole Miller <cole.miller@canonical.com>
Signed-off-by: Cole Miller <cole.miller@canonical.com>
Signed-off-by: Cole Miller <cole.miller@canonical.com>
@cole-miller cole-miller marked this pull request as ready for review October 8, 2024 16:52
@cole-miller
Copy link
Contributor Author

I just realized there's an issue I didn't deal with---if you connect to a node that happens to be the leader and open a DB with with the ALLOW_STALE flag, the connection will still be shut down when the node loses leadership. I'll fix that quickly...

Signed-off-by: Cole Miller <cole.miller@canonical.com>
Signed-off-by: Cole Miller <cole.miller@canonical.com>
Signed-off-by: Cole Miller <cole.miller@canonical.com>
Signed-off-by: Cole Miller <cole.miller@canonical.com>
@cole-miller
Copy link
Contributor Author

Pushed a new revision incorporating feedback from @marco6---the semantics of the OPEN flag have changed, in particular it now blocks modifications even when the server is currently the leader. This makes the behavior of connections using this flag more predictable and pushes clients to choose the right kind of connection for their use-case. The flag has been renamed from ALLOW_STALE to READONLY to reflect this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant